feat(runtime-interface-client): Add Lambda-Runtime-Invocation-Id support for cross-wiring protection#624
feat(runtime-interface-client): Add Lambda-Runtime-Invocation-Id support for cross-wiring protection#624vip-amzn wants to merge 1 commit into
Conversation
…ort for cross-wiring protection Echo the invocation ID received from RAPID on /next back on /response and /error, enabling RAPID to detect and reject stale responses from timed-out invocations. Fully backward compatible.
rudraroop
left a comment
There was a problem hiding this comment.
Looks good overall - my major point of feedback would be adding a test for the positive path where invocation IDs are non-null
Have added some casing/hardcoding related comments - whatever I could see
| headers.put(ERROR_TYPE_HEADER, error.errorType.getRapidError()); | ||
|
|
||
| if (invocationId != null) { | ||
| headers.put("Lambda-Runtime-Invocation-Id", invocationId); |
There was a problem hiding this comment.
the other headers in this file are declared as constants at the start of the file. I would suggest keeping that uniform and making a constant above for this one as well
| static constexpr auto DEADLINE_MS_HEADER = "lambda-runtime-deadline-ms"; | ||
| static constexpr auto FUNCTION_ARN_HEADER = "lambda-runtime-invoked-function-arn"; | ||
| static constexpr auto TENANT_ID_HEADER = "lambda-runtime-aws-tenant-id"; | ||
| static constexpr auto INVOCATION_ID_HEADER = "lambda-runtime-invocation-id"; |
There was a problem hiding this comment.
The "L" is capitalised in the java file but lowercase here
|
|
||
| LambdaError lambdaError = new LambdaError(errorRequest, rapidErrorType); | ||
| lambdaRuntimeApiClientImpl.reportInvocationError(requestId, lambdaError); | ||
| lambdaRuntimeApiClientImpl.reportInvocationError(requestId, lambdaError, null); |
There was a problem hiding this comment.
I think we should have one test for reportInvocationError which tests a positive path i.e. one where the Lambda-Runtime-Invocation-Id is actually getting passed instead of null
There was a problem hiding this comment.
Same feedback for positive path test inclusion as in this file LambdaRuntimeApiClientImplTest.java
| headers = curl_slist_append(headers, "transfer-encoding:"); | ||
| headers = curl_slist_append(headers, m_user_agent_header.c_str()); | ||
| if (!invocation_id.empty()) { | ||
| headers = curl_slist_append(headers, ("lambda-runtime-invocation-id: " + invocation_id).c_str()); |
There was a problem hiding this comment.
Should use the header constant here instead of hardcoding again?
Summary
Add
Lambda-Runtime-Invocation-Idheader support for cross-wiring protection.The RIC now echoes the invocation ID received from RAPID on
/nextback on/responseand/error, enabling RAPID to detect and reject stale responses from timed-out invocations.Problem
On Lambda Managed Instances (LMI) and On-Demand (OD), when an invoke times out, the runtime process continues running in the background. If a new invoke arrives with the same
requestId, RAPID accepts it. The still-running old invocation eventually posts its response, and RAPID matches it to the new invoke — delivering the wrong response (cross-wiring).Solution
RAPID sends a unique per-invoke identifier via
Lambda-Runtime-Invocation-Idheader on/next. The runtime echoes it back on/responseand/error. RAPID validates the match before accepting the response.Changes
aws-lambda-cpp-0.2.7/runtime.h: Addedinvocation_idfield toinvocation_request, added param topost_success/post_failure/do_postaws-lambda-cpp-0.2.7/runtime.cpp: Parse header inget_next(), add to curl request indo_post()when non-emptyNativeClient.cpp/.h: JNI readsinvocation_idfrom C++ response, passes to Java;postInvocationResponseaccepts nullable invocationIdInvocationRequest.java: NewinvocationIdfieldNativeClient.java: Updated native method signatureLambdaRuntimeApiClient.java/Impl:reportInvocationSuccess/reportInvocationErroraccept invocationIdAWSLambda.java: Thread invocationId through runtime loopBackward Compatibility
Fully backward compatible in both directions: